Prevent early wrapping for styled TUI messages by gtsiolis · Pull Request #111 · localstack/lstk

gtsiolis · 2026-03-12T16:36:08Z

Some CLI output wraps early, including Warning, Success, and Note messages.

BEFORE	AFTER

coderabbitai · 2026-03-12T16:40:59Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: a3323338-2bb7-4be6-9355-1feafac3354e

📥 Commits

Reviewing files that changed from the base of the PR and between 4491fe5 and 2f0b19d.

📒 Files selected for processing (3)

internal/ui/app.go
internal/ui/app_test.go
internal/ui/components/message.go

🚧 Files skipped from review as they are similar to previous changes (1)

internal/ui/app.go

📝 Walkthrough

Walkthrough

The UI now preserves message events on styled lines and renders them via a new wrapped-message renderer. Update stores message pointers in lines; View delegates rendering of message-backed lines to RenderWrappedMessage which supports width-based wrapping and multi-line indentation.

Changes

Cohort / File(s)	Summary
Message Event Storage & Handling `internal/ui/app.go`	Added `message *output.MessageEvent` to `styledLine`. Update stores a copied MessageEvent on lines and buffers/append logic updated; View detects non-nil `message` and invokes the message renderer instead of standard line rendering.
Message Rendering `internal/ui/components/message.go`	Added `RenderWrappedMessage(e output.MessageEvent, width int) string`. `RenderMessage` delegates to wrapped variant. `messagePrefix` signature changed to return `(prefixText, styledPrefix)`; rendering updated to produce wrapped, indented multi-line output and handle empty/zero-width cases.
Message Wrapping Tests `internal/ui/app_test.go`	Added `TestAppMessageEventWrapsOnVisibleWidth` and new test imports (lipgloss, termenv). Test asserts long message wraps at word boundaries when rendered at constrained width.

Sequence Diagram(s)

sequenceDiagram
    participant AppUpdate as App.Update()
    participant MsgEvt as output.MessageEvent
    participant StyledLine as styledLine
    participant AppView as App.View()
    participant Renderer as RenderWrappedMessage()
    participant Terminal as Terminal Display

    AppUpdate->>MsgEvt: receive MessageEvent
    MsgEvt-->>AppUpdate: event data
    AppUpdate->>StyledLine: copy & store pointer in message field
    StyledLine-->>AppUpdate: stored

    AppView->>StyledLine: inspect message field
    alt message != nil
        StyledLine-->>AppView: has message
        AppView->>Renderer: RenderWrappedMessage(event, width)
        Renderer->>Renderer: wrap text, add prefixes & indentation
        Renderer-->>AppView: formatted string
        AppView->>Terminal: render formatted message
    else message == nil
        StyledLine-->>AppView: no message
        AppView->>Terminal: render standard styled line
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Remove left indent from messages in TUI #68 — touches internal/ui message rendering/layout and likely overlaps with message rendering changes.

Suggested reviewers

carole-lavillonniere
silv-io
anisaoshafi

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main purpose of the changes: preventing early wrapping for styled TUI messages by implementing RenderWrappedMessage functionality and handling message wrapping at word boundaries.
Description check	✅ Passed	The description is related to the changeset, showing before/after terminal output demonstrating that styled messages (Success, Note) now wrap properly at word boundaries instead of wrapping early.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings (stacked PR)
📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch george/des-168

📝 Coding Plan

Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Tip

CodeRabbit can generate a title for your PR based on the changes.

Add @coderabbitai placeholder anywhere in the title of your PR and CodeRabbit will replace it with a title based on the changes in the PR. You can change the placeholder by changing the reviews.auto_title_placeholder setting.

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

internal/ui/app_test.go (1)

185-189: Consider removing t.Parallel() when modifying global lipgloss state.

lipgloss.SetColorProfile modifies global state, which can interfere with other parallel tests that depend on the color profile. While the cleanup restores the original value, there's a race window during execution.

Option 1: Remove t.Parallel()

 func TestAppMessageEventWrapsOnVisibleWidth(t *testing.T) {
-	t.Parallel()
-
 	original := lipgloss.ColorProfile()

Option 2: Keep as-is with a comment

 func TestAppMessageEventWrapsOnVisibleWidth(t *testing.T) {
 	t.Parallel()
 
+	// Note: SetColorProfile is global but other tests don't depend on a specific profile.
 	original := lipgloss.ColorProfile()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/ui/app_test.go` around lines 185 - 189, The test calls t.Parallel()
while changing global lipgloss state via lipgloss.SetColorProfile and restoring
it with t.Cleanup, which can race with other parallel tests; remove the call to
t.Parallel() in this test (or alternatively document the risk with a clear
comment if you must keep parallelism) so the sequence using
lipgloss.ColorProfile(), lipgloss.SetColorProfile(termenv.TrueColor) and
t.Cleanup(...) runs serially and cannot interfere with other tests.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/ui/components/message.go`:
- Around line 21-26: Adjust the width math to account for the single space
inserted between the prefix and the message: when checking whether the prefix
alone consumes the line, use width <= len([]rune(prefixText)) + 1, and compute
availableWidth := width - len([]rune(prefixText)) - 1 before calling
wrap.SoftWrap(e.Text, availableWidth); update any other uses that assume no
separator to use the same -1 adjustment (references: prefixText, availableWidth,
wrap.SoftWrap, prefix + " ", styles.Message.Render, e.Text).

---

Nitpick comments:
In `@internal/ui/app_test.go`:
- Around line 185-189: The test calls t.Parallel() while changing global
lipgloss state via lipgloss.SetColorProfile and restoring it with t.Cleanup,
which can race with other parallel tests; remove the call to t.Parallel() in
this test (or alternatively document the risk with a clear comment if you must
keep parallelism) so the sequence using lipgloss.ColorProfile(),
lipgloss.SetColorProfile(termenv.TrueColor) and t.Cleanup(...) runs serially and
cannot interfere with other tests.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: c7e0cfbd-8134-42c7-a631-292e11b19864

📥 Commits

Reviewing files that changed from the base of the PR and between 2ce11ae and 4491fe5.

📒 Files selected for processing (3)

internal/ui/app.go
internal/ui/app_test.go
internal/ui/components/message.go

internal/ui/components/message.go

anisaoshafi

LGTM! Thanks George 🧹

gtsiolis requested review from anisaoshafi, carole-lavillonniere and silv-io as code owners March 12, 2026 16:36

gtsiolis self-assigned this Mar 12, 2026

coderabbitai bot reviewed Mar 12, 2026

View reviewed changes

internal/ui/components/message.go Outdated Show resolved Hide resolved

Prevent early wrapping for styled TUI messages

2f0b19d

gtsiolis force-pushed the george/des-168 branch from 4491fe5 to 2f0b19d Compare March 12, 2026 17:37

anisaoshafi approved these changes Mar 13, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent early wrapping for styled TUI messages#111

Prevent early wrapping for styled TUI messages#111
gtsiolis wants to merge 1 commit intomainfrom
george/des-168

gtsiolis commented Mar 12, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Mar 12, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

anisaoshafi left a comment •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

gtsiolis commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

anisaoshafi left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gtsiolis commented Mar 12, 2026 •

edited

Loading

coderabbitai bot commented Mar 12, 2026 •

edited

Loading

anisaoshafi left a comment •

edited

Loading